SOLR-18093: Add top-level 'queries' support to JsonQueryRequest in SolrJ#4276
SOLR-18093: Add top-level 'queries' support to JsonQueryRequest in SolrJ#4276ercsonusharma wants to merge 11 commits intoapache:mainfrom
Conversation
github-actions
Bot
added
documentation
dsmiley
left a comment
dsmiley
left a comment
There was a problem hiding this comment.
Thanks for doing this :-)
There was a problem hiding this comment.
Mmmmm, not sure what I think about adding another handler to one of only 2 configSets we ship with Solr. But I suppose it's fine as this particular one, techproducts, is to show off features. So sure.
CC @epugh for 2nd opinion.
|
Fixed a flaky test in CombinedQuerySolrCloudTest.java from #4277. @dsmiley - When you have a moment, Would you be able to help here by running the workflows to validate this, and merge it once it passes? |
| </requestHandler> | ||
|
|
||
| <initParams path="/update/**,/query,/select,/tvrh,/elevate,/spell,update"> | ||
| <requestHandler name="/search" class="solr.CombinedQuerySearchHandler"> |
There was a problem hiding this comment.
I understand you are touching the techproducts config because it's necessary in order to demonstrate RRF. And you seem to have chosen "/search" because 2 other names "/select" and "/query" were chosen. If another feature were to come along, would yet another SearchHandler need to be defined for it with an attempt to find yet another synonym?
Two counter-proposals:
(A) Expand the scope of "/query". It's harmless to use CombinedQuerySearchHandler without any combining. CombinedQuerySearchHandler just means it's capable of combining.
(B) Use "/rrf" instead.
(C) The tutorial/documentation can show how to use our config API to manipulate the config at runtime by adding a handler and component (even in one request). And use a name like "/rrf". You can find this approach for the TaggerHandler: https://solr.apache.org/guide/solr/latest/query-guide/tagger-handler.html#create-and-configure-a-solr-collection
I prefer C. But not as-is with "/search"!
There was a problem hiding this comment.
Sure. renamed to /rrf. Will take up the documentation in a follow up change.
|
LMK when you think it's ready to merge. I think it's questionable to make the test put all data in one shard... but as long as the combiner is tested in other tests with data across shards, it's okay. |
|
Yes, the combiner is already tested in other tests with data across shards. |
3 participants
https://issues.apache.org/jira/browse/SOLR-18093
Description
Currently,
JsonQueryRequestin SolrJ supports specifying a single query via setQuery() and other top-level parameters like limit, sort, fields etc.However, it does not support top-level "queries" blocks, which is a good addition to fully leverage the combined query component and the Additional Queries feature in Solr’s JSON Query DSL. This limitation prevents constructing multiple queries or additional scoring/re-ranking queries in a single JSON request from the client side.
Solution
Added method to pass the required params in
JsonQueryRequest. Also, enhanced the documentation.Tests
Added Unit tests and Integration tests.
Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.